jwt-cpp: fix building without picojson #12735

JorgenPo · 2022-08-31T21:23:38Z

Specify library name and version: jwt-cpp/0.6.0

Fix compilation of jwt-cpp without picojson library
Fixes #12706

I've read the guidelines for contributing.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the conan-center hook activated.

CLAassistant · 2022-08-31T21:23:43Z

All committers have signed the CLA.

JorgenPo · 2022-08-31T21:28:56Z

@SpaceIm @prince-chrismc, not sure what to do with test_package, can you please clarify what is the right way to deal with it?

prince-chrismc · 2022-09-01T04:47:31Z

Make sure to comment in #4 🙏

prince-chrismc · 2022-09-01T04:48:44Z

Add the header #include "jwt-cpp/traits/kazuho-picojson/defaults.h" to the test package please

uilianries · 2022-09-01T07:48:13Z

@JorgenPo PLease, dont forget to sign the CLA pointed in this PR.

JorgenPo · 2022-09-01T19:22:10Z

@prince-chrismc, done!
@uilianries, signed the CLA

Signed-off-by: Uilian Ries <uilianries@gmail.com>

uilianries · 2022-09-02T08:55:42Z

@JorgenPo Please, consider JorgenPo#1

It will prepare jwt-cpp recipe for Conan 2.0

Adapt for Conan 2.0

JorgenPo · 2022-09-05T08:21:26Z

@uilianries, merged your PR

uilianries · 2022-09-05T14:42:12Z

recipes/jwt-cpp/all/conanfile.py

-            tools.patch(**patch)
+        get(self, **self.conan_data["sources"][self.version],
+            destination=self.source_folder, strip_root=True)
+        apply_conandata_patches(self)


Suggested change

apply_conandata_patches(self)

Applied suggested change

uilianries · 2022-09-05T14:42:59Z

recipes/jwt-cpp/all/conanfile.py

+        basic_layout(self, src_folder="src")
+
+    def build(self):
+        pass


Suggested change

pass

apply_conandata_patches(self)

Doing a new try, it works when using no_copy_source

prince-chrismc · 2022-09-06T17:19:43Z

recipes/jwt-cpp/all/conanfile.py

@@ -27,21 +27,26 @@ def export_sources(self):
    def requirements(self):
        self.requires("openssl/1.1.1q")
        if not self._supports_generic_json:
-            self.requires("picojson/1.3.0")
+            self.requires("picojson/cci.20210117")


I think it's better to point to official releases when they are available

Suggested change

self.requires("picojson/cci.20210117")

self.requires("picojson/cci.1.3.0")

prince-chrismc · 2022-09-06T17:21:41Z

recipes/jwt-cpp/all/test_package/test_package.cpp

@@ -1,4 +1,5 @@
 #include <jwt-cpp/jwt.h>
+#include <jwt-cpp/traits/kazuho-picojson/defaults.h>


this only exists in 0.6.0 +

You'll need to drop older versions 🤔

I added some additional definitions to the test package and placed picojson traits files to the test package folder to be able to compile older jwt-cpp versions.
Tested building on all versions listed in conandata.yml

How am I supposed to use this package now? Why do you want me to copy-paste these files as well, instead of just changing _supports_generic_json to version >= 6.0. I see no reason why we want to define JWT_DISABLE_PICOJSON in the recipe.

This is not an example, the test package is more complicated because it's supporting many versions

If you use version 0.6.0 you can include any of the support JSON libraries and that file will already be present

I updated 0.5.0 to new revision and it became not usable because of hard coded define :(
I had to upgrade to 0.6.0 and slightly adjust my code, now it is working. But I still think setting this define without option flag was a bad decision. It turns off part of the functionality of the library which I was using in the previous revision.

https://github.com/Thalhammer/jwt-cpp/releases did add support for generic json at 0.5.0 but I agree this could be improved #12735 (review)

Feel free to open a PR to improve the recipe :)

prince-chrismc · 2022-09-06T18:15:54Z

This PR is trying to do a lot 🙈 sorry everything is getting tossed in

JorgenPo · 2022-09-12T07:20:06Z

@prince-chrismc, @uilianries, fixed all review comments, could you have a look please?

uilianries · 2022-09-12T15:27:36Z

@JorgenPo Waiting for the CI result ... It's taking more time usual. Your PR looks good now.

conan-center-bot · 2022-09-14T04:03:28Z

All green in build 5 (1ffa1868474750bbfbab7aea9ebdbfe2ec9c1ad1):

jwt-cpp/0.6.0@:
All packages built successfully! (All logs)
🔸 Informative: This recipe is not ready for Conan v2

_{We have started the migration process to Conan v2 and exporting recipes successfully will be required in the future.

This is just an informative note to gain awareness about the process, no need to take any action. The plan is to enforce smaller steps that are easier to fix and, eventually, this conan export step will work.

See the recipe migration guide to know more about the changes required.}
```
WARN: Name containing special chars is discouraged 'jwt-cpp'
jwt-cpp/0.6.0: Exporting package recipe
jwt-cpp/0.6.0 exports: File 'conandata.yml' found. Exporting it...
jwt-cpp/0.6.0: Copied 1 '.yml' file: conandata.yml
jwt-cpp/0.6.0 exports: Copied 1 '.yml' file: conandata.yml
jwt-cpp/0.6.0: Calling export_sources()
ERROR: jwt-cpp/0.6.0: Error in export_sources() method, line 25
	self.copy(patch["patch_file"])
	AttributeError: 'JwtCppConan' object has no attribute 'copy'
```
jwt-cpp/0.5.0@:
All packages built successfully! (All logs)
🔸 Informative: This recipe is not ready for Conan v2

_{We have started the migration process to Conan v2 and exporting recipes successfully will be required in the future.

This is just an informative note to gain awareness about the process, no need to take any action. The plan is to enforce smaller steps that are easier to fix and, eventually, this conan export step will work.

See the recipe migration guide to know more about the changes required.}
```
WARN: Name containing special chars is discouraged 'jwt-cpp'
jwt-cpp/0.5.0: Exporting package recipe
jwt-cpp/0.5.0 exports: File 'conandata.yml' found. Exporting it...
jwt-cpp/0.5.0: Copied 1 '.yml' file: conandata.yml
jwt-cpp/0.5.0 exports: Copied 1 '.yml' file: conandata.yml
jwt-cpp/0.5.0: Calling export_sources()
ERROR: jwt-cpp/0.5.0: Error in export_sources() method, line 25
	self.copy(patch["patch_file"])
	AttributeError: 'JwtCppConan' object has no attribute 'copy'
```
jwt-cpp/0.5.1@:
All packages built successfully! (All logs)
🔸 Informative: This recipe is not ready for Conan v2

_{We have started the migration process to Conan v2 and exporting recipes successfully will be required in the future.

This is just an informative note to gain awareness about the process, no need to take any action. The plan is to enforce smaller steps that are easier to fix and, eventually, this conan export step will work.

See the recipe migration guide to know more about the changes required.}
```
WARN: Name containing special chars is discouraged 'jwt-cpp'
jwt-cpp/0.5.1: Exporting package recipe
jwt-cpp/0.5.1 exports: File 'conandata.yml' found. Exporting it...
jwt-cpp/0.5.1: Copied 1 '.yml' file: conandata.yml
jwt-cpp/0.5.1 exports: Copied 1 '.yml' file: conandata.yml
jwt-cpp/0.5.1: Calling export_sources()
ERROR: jwt-cpp/0.5.1: Error in export_sources() method, line 25
	self.copy(patch["patch_file"])
	AttributeError: 'JwtCppConan' object has no attribute 'copy'
```
jwt-cpp/0.4.0@:
All packages built successfully! (All logs)
🔸 Informative: This recipe is not ready for Conan v2

_{We have started the migration process to Conan v2 and exporting recipes successfully will be required in the future.

This is just an informative note to gain awareness about the process, no need to take any action. The plan is to enforce smaller steps that are easier to fix and, eventually, this conan export step will work.

See the recipe migration guide to know more about the changes required.}
```
WARN: Name containing special chars is discouraged 'jwt-cpp'
jwt-cpp/0.4.0: Exporting package recipe
jwt-cpp/0.4.0 exports: File 'conandata.yml' found. Exporting it...
jwt-cpp/0.4.0: Copied 1 '.yml' file: conandata.yml
jwt-cpp/0.4.0 exports: Copied 1 '.yml' file: conandata.yml
jwt-cpp/0.4.0: Calling export_sources()
ERROR: jwt-cpp/0.4.0: Error in export_sources() method, line 25
	self.copy(patch["patch_file"])
	AttributeError: 'JwtCppConan' object has no attribute 'copy'
```
jwt-cpp/0.3.1@:
All packages built successfully! (All logs)
🔸 Informative: This recipe is not ready for Conan v2

_{We have started the migration process to Conan v2 and exporting recipes successfully will be required in the future.

This is just an informative note to gain awareness about the process, no need to take any action. The plan is to enforce smaller steps that are easier to fix and, eventually, this conan export step will work.

See the recipe migration guide to know more about the changes required.}
```
WARN: Name containing special chars is discouraged 'jwt-cpp'
jwt-cpp/0.3.1: Exporting package recipe
jwt-cpp/0.3.1 exports: File 'conandata.yml' found. Exporting it...
jwt-cpp/0.3.1: Copied 1 '.yml' file: conandata.yml
jwt-cpp/0.3.1 exports: Copied 1 '.yml' file: conandata.yml
jwt-cpp/0.3.1: Calling export_sources()
ERROR: jwt-cpp/0.3.1: Error in export_sources() method, line 25
	self.copy(patch["patch_file"])
	AttributeError: 'JwtCppConan' object has no attribute 'copy'
```

prince-chrismc

I am not crazy for copy pasting files that exist but the solution is pretty complicated and we want this for 2.0

I'll circle back to work on it

* jwt-cpp: fix building without picojson * jwt-cpp: add missing traits header to test package * Adapt for Conan 2.0 Signed-off-by: Uilian Ries <uilianries@gmail.com> * Fix building test_package for older jwt-cpp versions * Conanfile review fixes * Add endline to the end of files Signed-off-by: Uilian Ries <uilianries@gmail.com> Co-authored-by: Uilian Ries <uilianries@gmail.com>

This comment has been minimized.

Sign in to view

conan-center-bot added the User-approval pending label Aug 31, 2022

github-actions bot mentioned this pull request Aug 31, 2022

Pull Requests Ready to be Reviewed prince-chrismc/conan-center-index-pending-review#1

Open

jwt-cpp: fix building without picojson

43a3a19

JorgenPo mentioned this pull request Sep 1, 2022

[!!] Access request #4

Closed

This comment has been minimized.

Sign in to view

JorgenPo and others added 2 commits September 1, 2022 22:19

jwt-cpp: add missing traits header to test package

400d5ff

Adapt for Conan 2.0

1dab96d

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Merge pull request #1 from uilianries/package/jwt-cpp

4840c98

Adapt for Conan 2.0

This comment has been minimized.

Sign in to view

uilianries reviewed Sep 5, 2022

View reviewed changes

prince-chrismc reviewed Sep 6, 2022

View reviewed changes

JorgenPo added 2 commits September 7, 2022 11:17

Fix building test_package for older jwt-cpp versions

86ffcf0

Conanfile review fixes

87905ac

conan-center-bot removed the User-approval pending label Sep 7, 2022

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Sep 7, 2022

Add endline to the end of files

1ffa186

conan-center-bot removed the Failed label Sep 8, 2022

prince-chrismc mentioned this pull request Sep 9, 2022

Conan 2.0 Migration Effort #12888

Closed

uilianries approved these changes Sep 14, 2022

View reviewed changes

prince-chrismc approved these changes Sep 14, 2022

View reviewed changes

conan-center-bot merged commit f8181c5 into conan-io:master Sep 14, 2022

JorgenPo deleted the package/jwt-cpp branch September 16, 2022 08:34

SpaceIm mentioned this pull request Aug 29, 2024

(#25070) jwt-cpp: Fix missing picojson header #25077

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

jwt-cpp: fix building without picojson #12735

jwt-cpp: fix building without picojson #12735

JorgenPo commented Aug 31, 2022 •

edited

Loading

CLAassistant commented Aug 31, 2022 •

edited

Loading

This comment has been minimized.

JorgenPo commented Aug 31, 2022

prince-chrismc commented Sep 1, 2022

prince-chrismc commented Sep 1, 2022 •

edited

Loading

uilianries commented Sep 1, 2022

This comment has been minimized.

JorgenPo commented Sep 1, 2022

uilianries commented Sep 2, 2022

This comment has been minimized.

JorgenPo commented Sep 5, 2022

uilianries Sep 5, 2022

JorgenPo Sep 7, 2022

uilianries Sep 5, 2022

JorgenPo Sep 7, 2022

prince-chrismc Sep 6, 2022

JorgenPo Sep 7, 2022

prince-chrismc Sep 6, 2022

JorgenPo Sep 7, 2022

technic Jan 4, 2023

prince-chrismc Jan 5, 2023

prince-chrismc Jan 5, 2023

technic Jan 5, 2023

prince-chrismc Jan 5, 2023

prince-chrismc commented Sep 6, 2022

This comment has been minimized.

JorgenPo commented Sep 12, 2022

uilianries commented Sep 12, 2022

conan-center-bot commented Sep 14, 2022

prince-chrismc left a comment

	self.requires("picojson/cci.20210117")
	self.requires("picojson/cci.1.3.0")

		@@ -1,4 +1,5 @@
		#include <jwt-cpp/jwt.h>
		#include <jwt-cpp/traits/kazuho-picojson/defaults.h>

jwt-cpp: fix building without picojson #12735

jwt-cpp: fix building without picojson #12735

Conversation

JorgenPo commented Aug 31, 2022 • edited Loading

CLAassistant commented Aug 31, 2022 • edited Loading

This comment has been minimized.

JorgenPo commented Aug 31, 2022

prince-chrismc commented Sep 1, 2022

prince-chrismc commented Sep 1, 2022 • edited Loading

uilianries commented Sep 1, 2022

This comment has been minimized.

JorgenPo commented Sep 1, 2022

uilianries commented Sep 2, 2022

This comment has been minimized.

JorgenPo commented Sep 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

prince-chrismc commented Sep 6, 2022

This comment has been minimized.

JorgenPo commented Sep 12, 2022

uilianries commented Sep 12, 2022

conan-center-bot commented Sep 14, 2022

prince-chrismc left a comment

Choose a reason for hiding this comment

JorgenPo commented Aug 31, 2022 •

edited

Loading

CLAassistant commented Aug 31, 2022 •

edited

Loading

prince-chrismc commented Sep 1, 2022 •

edited

Loading